Skip to content

gh-145335: Fix os functions when passing fd -1 as path - #145439

Merged
vstinner merged 5 commits into
python:mainfrom
vstinner:ebadf
Mar 3, 2026
Merged

gh-145335: Fix os functions when passing fd -1 as path#145439
vstinner merged 5 commits into
python:mainfrom
vstinner:ebadf

Conversation

@vstinner

@vstinner vstinner commented Mar 2, 2026 •
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Member

os.listdir(-1) and os.scandir(-1) now fail with OSError(errno.EBADF) rather than listing the current directory.

os.listxattr(-1) now fails with OSError(errno.EBADF) rather than listing extended attributes of the current directory.

Victor Stinner
pythongh-145335: Fix os functions when passing fd -1 as path
d57be54 
os.listdir(-1) and os.scandir(-1) now fail with OSError(errno.EBADF)
rather than listing the current directory.

os.listxattr(-1) now fails with OSError(errno.EBADF) rather than
listing extended attributes of the current directory.
@bedevere-app bedevere-app Bot mentioned this pull request Mar 2, 2026
Closed
@vstinner vstinner mentioned this pull request Mar 2, 2026
Merged
@vstinner vstinner added 🔨 test-with-buildbots Test PR w/ buildbots; report in status section and removed 🔨 test-with-buildbots Test PR w/ buildbots; report in status section labels Mar 2, 2026
@bedevere-bot

ghost commented Mar 2, 2026

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @vstinner for commit a894262 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F145439%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Mar 2, 2026
@vstinner

ghost commented Mar 3, 2026

Copy link
Copy Markdown
Member Author

Tests failed on 5 buildbots, but failures are unrelated to this change.

buildbot/AMD64 CentOS9 NoGIL Refleaks PR

FAIL: test_free_reference (test.test_concurrent_futures.test_thread_pool.ThreadPoolExecutorTest.test_free_reference)

buildbot/AMD64 Fedora Stable Clang Installed PR
buildbot/PPC64LE Fedora Stable Clang Installed PR
buildbot/s390x Fedora Stable Clang Installed PR
buildbot/x86 Debian Installed with X PR

3 tests failed: test_datetime test_import test_interpreters. It should be fixed by commit 46c5c57.

@vstinner
vstinner marked this pull request as ready for review March 3, 2026 10:27
@vstinner

ghost commented Mar 3, 2026 •
edited by vstinner
Loading

Copy link
Copy Markdown
Member Author

The PR is now ready for review.

cc @picnixz @aisk

picnixz
picnixz reviewed Mar 3, 2026
View reviewed changes
Comment thread Misc/NEWS.d/next/Library/2026-03-02-20-08-09.gh-issue-145335.lVTBvd.rst
picnixz
picnixz reviewed Mar 3, 2026
View reviewed changes
Comment thread Lib/test/test_os/test_os.py
picnixz
picnixz approved these changes Mar 3, 2026
View reviewed changes

ghost left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Are there actually other places where we use the path converter and that would be useful? (that is, do you plan to look at other modules as follow-up?)

@vstinner
vstinner merged commit 52c8efa into python:main Mar 3, 2026
@vstinner
vstinner deleted the ebadf branch March 3, 2026 12:57
@vstinner

ghost commented Mar 3, 2026

Copy link
Copy Markdown
Member Author

Merged. Thanks for the review!

@vstinner

ghost commented Mar 3, 2026

Copy link
Copy Markdown
Member Author

@picnixz:

Are there actually other places where we use the path converter and that would be useful? (that is, do you plan to look at other modules as follow-up?)

I had a look and I didn't find other places with the same bug.

  • path_converter() and path_t are specific to posixmodule.c.
  • PyObject_AsFileDescriptor() fails with ValueError if the file descriptor is negative, so check for errors using fd == -1 or fd < 0 is correct.
  • _PyLong_FileDescriptor_Converter() calls PyObject_AsFileDescriptor().
  • Argument Clinic generates PyObject_AsFileDescriptor() calls when using fildes type.
  • open() calls FileIO() which also explicitly rejects negative file descriptors.

@freakboy3742

ghost commented Mar 5, 2026

Copy link
Copy Markdown
Contributor

This appears to have broken Emscripten builds; not sure if the fix is a test skip, or if there's some additional handling required.

/cc @hoodmane

@freakboy3742 freakboy3742 mentioned this pull request Mar 5, 2026
Merged
@hoodmane

ghost commented Mar 5, 2026

Copy link
Copy Markdown
Contributor

I'll look into it, thanks for the ping @freakboy3742.

@hoodmane

ghost commented Mar 5, 2026

Copy link
Copy Markdown
Contributor

Problem seems to be that on Emscripten execve is "supported" but always returns ENOEXEC.

@hoodmane

ghost commented Mar 5, 2026

Copy link
Copy Markdown
Contributor

Fix here: #145528

@vstinner

ghost commented Mar 5, 2026

Copy link
Copy Markdown
Member Author

This appears to have broken Emscripten builds; not sure if the fix is a test skip, or if there's some additional handling required.

Oh, I missed this buildbot failure since it's still classified as Unstable. I propose to promote it to Stable: python/buildmaster-config#678.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@picnixz picnixz picnixz approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@vstinner @bedevere-bot @freakboy3742 @hoodmane @picnixz